Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

compiler: disallow callconv etc from depending on function parameters #22264

Merged
merged 4 commits into from
Dec 19, 2024
Merged

compiler: disallow callconv etc from depending on function parameters #22264

merged 4 commits into from
Dec 19, 2024

Conversation

mlugg
Copy link
Member

@mlugg mlugg commented Dec 18, 2024 •
edited
Loading

even if you thought this feature was useful it really doesn't seem worth 500 damn lines of compiler logic :)

Resolves: #22261

This PR also implements a breaking change to disallow align/linksection/addrspace annotations on globals with comptime-only types. This includes function aliases, but not function declarations.

Rexicon226
Rexicon226 reviewed Dec 18, 2024
View reviewed changes
src/Zcu/PerThread.zig Outdated Show resolved Hide resolved
@mlugg
compiler: move RuntimeIndex to Sema
242bb44 
Just a small refactor.
@mlugg mlugg force-pushed the no-generic-callconv branch from 5e48cdc to 594ef31 Compare December 18, 2024 20:35
@mlugg mlugg added breaking Implementing this issue could cause existing code to no longer compile or have different behavior. release notes This PR should be mentioned in the release notes. labels Dec 18, 2024
@mlugg mlugg force-pushed the no-generic-callconv branch from 594ef31 to d25df63 Compare December 18, 2024 22:08
@mlugg mlugg force-pushed the no-generic-callconv branch 2 times, most recently from 1679a65 to 26aafac Compare December 18, 2024 23:35
mlugg added 2 commits December 19, 2024 03:21
@mlugg
compiler: disallow align etc annotations on comptime-only globals
eac87ea 
This includes function aliases, but not function declarations.

Also, re-introduce a target check for function alignment which was
inadvertently removed in the prior commit.
@mlugg
Zcu: allow node_offset_var_decl_* source location for function decl…
58b8b1a 
…arations
@mlugg mlugg force-pushed the no-generic-callconv branch from 26aafac to 58b8b1a Compare December 19, 2024 03:22
@mlugg mlugg enabled auto-merge December 19, 2024 04:29
@mlugg mlugg merged commit e2e3633 into ziglang:master Dec 19, 2024
10 checks passed
@andrewrk
Copy link
Member 

Benchmark 1 (8 runs): stage3-4ae101ca8/bin/zig test ../lib/std/std.zig -fno-emit-bin
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          8.03s  ± 71.3ms    7.89s  … 8.15s           2 (25%)        0%
  peak_rss            590MB ±  339KB     589MB …  590MB          0 ( 0%)        0%
  cpu_cycles         42.3G  ± 96.5M     42.2G  … 42.4G           0 ( 0%)        0%
  instructions       83.0G  ± 16.0K     83.0G  … 83.0G           1 (13%)        0%
  cache_references   3.28G  ± 9.91M     3.27G  … 3.30G           0 ( 0%)        0%
  cache_misses       82.6M  ± 2.73M     79.9M  … 88.7M           1 (13%)        0%
  branch_misses      59.6M  ±  534K     59.0M  … 60.7M           1 (13%)        0%
Benchmark 2 (8 runs): stage3-e2e363361/bin/zig test ../lib/std/std.zig -fno-emit-bin
  measurement          mean ± σ            min … max           outliers         delta
  wall_time          7.93s  ±  149ms    7.70s  … 8.06s           0 ( 0%)          -  1.3% ±  1.6%
  peak_rss            589MB ±  851KB     588MB …  590MB          0 ( 0%)          -  0.1% ±  0.1%
  cpu_cycles         42.3G  ± 97.0M     42.2G  … 42.5G           1 (13%)          +  0.0% ±  0.2%
  instructions       83.0G  ±  406K     83.0G  … 83.0G           1 (13%)          -  0.0% ±  0.0%
  cache_references   3.28G  ± 10.4M     3.27G  … 3.29G           0 ( 0%)          -  0.2% ±  0.3%
  cache_misses       89.8M  ± 1.94M     87.1M  … 92.2M           0 ( 0%)        💩+  8.7% ±  3.1%
  branch_misses      59.7M  ±  579K     59.0M  … 60.5M           0 ( 0%)          +  0.2% ±  1.0%

@mlugg
Copy link
Member Author

mlugg commented Dec 19, 2024

That's a fascinating comparison. I suppose I'm introducing a little more indirection via function instances' generic owners, which are more likely to miss, or something along those lines? (Regardless, no overall perf impact -- & an improvement if anything -- so pretty good.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Rexicon226 Rexicon226 Rexicon226 left review comments

Assignees
No one assigned
Labels
breaking Implementing this issue could cause existing code to no longer compile or have different behavior. release notes This PR should be mentioned in the release notes.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Do not allow callconv, align, addrspace, or linksection to reference function arguments
3 participants
@mlugg @andrewrk @Rexicon226